fix(engine): scope the tracked-set cast filter context to the published set - #7035
Conversation
…ed set
Addresses three review findings on this PR.
1. `tracked_set_cast_candidates` stopped reading `ability.targets` for its
candidates but still built the `FilterContext` from the same unscoped
ability, so a filter leg that reads object scope (a ParentTarget-relative
comparison, a same-name or shares-a-type leg) would still have evaluated
against the injected reveal window. Same defect class as the bug this PR
fixes, one layer down. Latent today — all 51 cards bind `filter: Any`,
which reads no object scope — and both sibling sites in this resolver
already clone and rescope, one of them under the same CR 607.2a.
2. CR 614.6 -> CR 607.2a at assembly.rs:2434. Missed in the previous commit
because the citation sweep used a hand-listed file set that omitted
assembly.rs.
3. `uncaused_exilers` named five shapes in its doc and pinned two. Add
`Dig{destination: Exile}`, `ExileHaunting` and
`ExileResolvingSpellInsteadOfGraveyard` so every named shape carries both
the positive chain_clause_is_exile_producer check and the negative
publishes_exiled_cause_at_resolution / this_way_cause_for_effect checks.
All three confirm as uncaused.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughTracked-set cast candidates are now deduplicated before filtering. Filter evaluation uses only the deduplicated published members. A tracked-set rules citation and parser test fixtures were also updated. ChangesTracked-set cast filtering
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Parse changes introduced by this PRBaseline pending for |
Follow-up to #7034. Three CodeRabbit findings were posted on that PR, but the merge queue landed it before the fixes could be pushed — queued branches can't be updated, and by the time I dequeued it had already merged. These are those fixes, unchanged in substance from what was verified against #7034's head.
1. Scope the filter context to the published set (Major, correctness)
tracked_set_cast_candidatesstopped readingability.targetsfor its candidates but still built theFilterContextfrom the same unscoped ability.FilterContext::from_abilitycarriesability.targets— for Sanar, the whole reveal window the chain seam injected — so a filter leg that reads object scope (aParentTarget-relative comparison, a same-name or shares-a-type leg) would still have evaluated against the injected window rather than the members actually published.Same defect class as the bug #7034 fixed, one layer down.
Latent, not live: all 51 tracked-set cast cards bind
filter: Any, which reads no object scope, so nothing misbehaves today. It is closed so it stays that way. Both sibling sites in this same resolver already clone and rescope the ability before constructing the context — one of them under the sameCR 607.2a. This makes the third site consistent with them.2.
CR 614.6→CR 607.2aatassembly.rs:2434Missed in #7034's citation sweep because that sweep used a hand-listed file set which omitted
assembly.rs.614.6 is "if an event is replaced, it never happens" and does not describe this code. 607.2a is the linked-ability rule for an activated or triggered ability that instructs a player to exile, which is what this block narrows against.
3. Pin the three unpinned uncaused exile shapes
uncaused_exilersinexiled_cause_publishers_all_stamp_exiled_at_runtimenamed five shapes in its doc comment and pinned two. AddsDig { destination: Some(Exile) },ExileHauntingandExileResolvingSpellInsteadOfGraveyard.Each now carries both the positive
chain_clause_is_exile_producercheck and the negativepublishes_exiled_cause_at_resolution/this_way_cause_for_effectchecks. All three confirm as genuinely uncaused — the test passes with them added, which is what establishes that a cause-filtered anaphor must not be bound after any of them.Verification
exiled_cause_publishers_all_stamp_exiled_at_runtimegreen with all five shapes pinnedissue_4253_sanar_vivid×7,urza_lord_high_artificer×2), covering thecast_from_zonebehavioural changemain(14b11cf5bf) with no conflictsSummary by CodeRabbit
Bug Fixes
Tests
Documentation